-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove RichEmbed in favour of MessageEmbed #1584
Conversation
src/structures/MessageEmbed.js
Outdated
name: data.provider.name, | ||
url: data.provider.url, | ||
} : null; | ||
this.provder = data.provider; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are unit tests, right? Won't they catch it?
can we just get rid of addBlankField |
* @param {boolean} [inline=false] Set the field to display inline | ||
* @returns {MessageEmbed} This embed | ||
*/ | ||
addBlankField(inline = false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Its such a useful small thing! Ain't nobody got time to get the unicode code for the 0-width space
* remove RichEmbed in favour of MessageEmbed * fix provider typo
From #1529, MessageEmbeds are now identical to RichEmbeds (with a few differences).
There's no further reason in having separate MessageEmbed and RichEmbed classes, as they're similar enough that you could require MessageEmbed inside of RichEmbed and export it to get identical behaviour.
This simplifies internal usage as well. Typechecking #1451 for RichEmbed and MessageEmbed could be simplified to just MessageEmbed.
In this commit:
<MessageEmbed>._apiTransform()
private method added so that internal variables can continue using camel case.<MessageEmbed>.color
getter returns null if colour is not set instead of throwing an error.<MessageEmbed>.createdAt
getter returns null if timestamp is unset/invalid.<RichEmbed>.addBlankField
added to MessageEmbed.Most properties that used to to be set to null when missing now default to undefined, to be more consistent with current RichEmbed behaviour.
Semantic versioning classification: